-
Notifications
You must be signed in to change notification settings - Fork 1
Geant4 parser code decompositon #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the Geant4 parser code into a more modular architecture by decomposing the monolithic Geant4MacroGenerator class and parser methods into separate builder and parser modules. The refactoring improves code organization and maintainability.
Key Changes:
- Extracted GDML generation logic into separate
gdml/module with dedicated builders for materials, solids, and structure - Decomposed macro generation into separate parsers for beam, scoring, histograms, run, and results in
macro/module - Moved particle and quantity mappings to a centralized
constants.pyfile
Reviewed changes
Copilot reviewed 13 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
converter/geant4/parser.py |
Simplified to delegate GDML and macro generation to new builder classes |
converter/geant4/constants.py |
Centralized GEANT4_PARTICLE_MAP and GEANT4_QUANTITY_MAP constants |
converter/geant4/gdml/builder.py |
Main GDML generation orchestrator with XML prettification |
converter/geant4/gdml/materials.py |
Material collection and emission logic |
converter/geant4/gdml/solids.py |
Solid geometry element generation |
converter/geant4/gdml/structure.py |
Structure and volume hierarchy generation |
converter/geant4/macro/builder.py |
Main macro generation orchestrator |
converter/geant4/macro/beam_parser.py |
Beam configuration parsing |
converter/geant4/macro/scoring_parser.py |
Scoring detector and quantity parsing |
converter/geant4/macro/histogram_parser.py |
Histogram output generation for probes |
converter/geant4/macro/run_parser.py |
Run command generation |
converter/geant4/macro/result_parser.py |
Result collection commands |
converter/geant4/Geant4MacroGenerator.py |
Removed in favor of modular parsers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
A general remark - why a simple code, like in Geant4GDMLBuilder is not a method ?
should do the job as well. The same applies to The code looks very weird and not pythonic :/ |
Fair enough, on daily basis i write in java so the code could have some OOP similarities,. Do you want more method base approach ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 16 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
grzanka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend to avoid as much as possible a coding style where you have a function which modify the parameters they take.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 18 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
grzanka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you convert all of your codes into simple functions, with meaningful names, which do not modify its arguments. Good idea would be also to completely get rid of classes.
We don't need a polymorphism here, nor any need to keep a state of some object. The logic here is simple transformation from one state to another which can be achieved by set of testable functions scattered among couple of modules.
we have only one class that is absolutely needed, ( extends class parser as every other converter ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 18 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Ciorolla any news ? |
|
It looks much more readable now, I can now proceed with E2E testing on yap-dev. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>

This pull request introduces a modular refactor and feature expansion for the Geant4 converter, focusing on the generation of GDML files and the organization of Geant4 constants. The main changes include moving large constant maps to a dedicated file, adding a new GDML builder with supporting modules for materials, solids, and structure, and cleaning up the macro generator accordingly.
Refactoring and Code Organization:
GEANT4_PARTICLE_MAP,GEANT4_QUANTITY_MAP, and related constants fromGeant4MacroGenerator.pyinto a newconstants.pyfile, improving code organization and reusability. (converter/geant4/constants.py)Geant4MacroGenerator.pyto remove in-file constants and instead import them from the newconstants.py, resulting in a cleaner and more maintainable macro generator. (converter/geant4/Geant4MacroGenerator.py) [1] [2]GDML Generation Feature:
gdml/builder.py) that generates GDML XML for a given geometry or an empty world, encapsulating the entry point and XML generation logic. (converter/geant4/gdml/builder.py)materials.pyto collect and emit material definitions, including special handling for "BlackHole". (converter/geant4/gdml/materials.py)solids.pyto create GDML solid elements based on geometry type. (converter/geant4/gdml/solids.py)structure.pyto recursively build the GDML structure and volume hierarchy. (converter/geant4/gdml/structure.py)